Skip to content

feat: flow inc query terminal metrics transport#8045

Merged
killme2008 merged 11 commits into
mainfrom
flow-inc-pr2b-terminal-metrics-transport
May 11, 2026
Merged

feat: flow inc query terminal metrics transport#8045
killme2008 merged 11 commits into
mainfrom
flow-inc-pr2b-terminal-metrics-transport

Conversation

@discord9
Copy link
Copy Markdown
Contributor

@discord9 discord9 commented Apr 28, 2026

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This PR wires terminal record-batch metrics through the Flight/client/Flow consumer path.

It builds on #8015, which introduced RecordBatchMetrics.region_watermarks and query-side terminal metric collection.

Changes

  • Adds Flight transport support for optional trailing terminal Metrics messages.
  • Adds client-side OutputWithMetrics / OutputMetrics helpers so callers can consume terminal metrics without breaking existing Output APIs.
  • Adds Flow frontend-client support for requesting and reading terminal region watermarks in both standalone and distributed modes.
  • Validates Flow query extensions at the Flight boundary before dispatch.
  • Keeps normal query/affected-row behavior backward-compatible when terminal metrics are absent.

Scope

This PR is limited to terminal metrics transport and consumption:

  • src/servers/src/grpc/flight.rs
  • src/client/src/database.rs
  • src/flow/src/batching_mode/frontend_client.rs
  • related error/test updates

It intentionally does not include later stale-cursor, incremental-after-seq, benchmark.

Compatibility

Existing client APIs (sql, query, create, alter, etc.) continue to return plain Output.
Terminal metrics are opt-in through the new metrics-aware helper path.

Malformed terminal metrics are rejected as transport/parsing errors instead of being silently ignored.

Tests

  • cargo test -p client terminal_metrics --lib
  • cargo test -p flow query_with_terminal_metrics --lib

Coverage includes:

  • stream terminal metrics roundtrip
  • affected-rows terminal metrics roundtrip
  • invalid terminal metrics rejection
  • invalid Flow extension rejection
  • standalone and distributed Flow consumer paths

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@github-actions github-actions Bot added size/M docs-not-required This change does not impact docs. labels Apr 28, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces OutputMetrics and OutputWithMetrics to track and expose terminal metrics, such as region watermarks, from query results across the client, frontend, and gRPC layers. It updates the gRPC Flight stream handling to support interleaved metrics messages and adds new API methods to retrieve these metrics. A critical issue was identified in the client's stream processing where a valid RecordBatch could be dropped if a subsequent Metrics message is malformed; yielding the batch before processing the next message is recommended to prevent data loss.

Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs Outdated
Comment thread src/servers/src/grpc/flight.rs Outdated
Comment thread src/servers/src/grpc/flight.rs Outdated
@github-actions github-actions Bot added size/L and removed size/M labels Apr 29, 2026
@discord9 discord9 marked this pull request as ready for review April 29, 2026 09:04
@discord9 discord9 requested review from a team and waynexia as code owners April 29, 2026 09:04
@discord9 discord9 requested a review from killme2008 April 29, 2026 09:17
@github-actions github-actions Bot added size/XL and removed size/L labels Apr 30, 2026
Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs
Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs
Comment thread src/common/grpc/src/flight.rs
Comment thread src/datanode/src/region_server.rs Outdated
Comment thread src/flow/src/batching_mode/frontend_client.rs Outdated
Comment thread src/query/src/datafusion.rs
Comment thread src/query/src/metrics.rs
Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs
Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs Outdated
Comment thread src/client/src/database.rs Outdated
Comment thread src/common/grpc/src/flight.rs
Comment thread src/flow/src/batching_mode/frontend_client.rs Outdated
Comment thread src/query/src/options.rs
Comment thread src/servers/src/grpc/flight.rs Outdated
discord9 added 9 commits May 11, 2026 10:34
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Avoid routing Flow-specific query extensions through comma-separated hints so checkpoint JSON values remain intact over Flight.

Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
@discord9 discord9 force-pushed the flow-inc-pr2b-terminal-metrics-transport branch from 665b034 to 4cef82b Compare May 11, 2026 02:36
Copy link
Copy Markdown
Member

@killme2008 killme2008 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@killme2008 killme2008 enabled auto-merge May 11, 2026 11:12
@killme2008 killme2008 added this pull request to the merge queue May 11, 2026
Merged via the queue into main with commit a282b20 May 11, 2026
46 of 47 checks passed
@killme2008 killme2008 deleted the flow-inc-pr2b-terminal-metrics-transport branch May 11, 2026 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-not-required This change does not impact docs. size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants